feat(backend): Signal support for handshake nonce #5905
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 87cf34e The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for signaling the handshake nonce flow by including a query parameter.
- In handshake.ts, the query parameter SupportsHandshakeNonce is appended to the URL.
- In handshake.test.ts, corresponding tests ensure the parameter is correctly set in both regular and development modes.
- In constants.ts, a new constant for SupportsHandshakeNonce is added to support the new query parameter.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/backend/src/tokens/handshake.ts | Appends a new query parameter for handshake nonce support. |
| packages/backend/src/tokens/tests/handshake.test.ts | Adds tests to validate the presence of the new query parameter. |
| packages/backend/src/constants.ts | Introduces the SupportsHandshakeNonce constant to hold the parameter key. |
|
LGTM. So to be doubly clear: this indicates "nonce support", but FAPI still has the option to return 'optimized' payload in initial response, right? |
|
@jfoshee Yeah, this is just to signal to the API that it COULD send a handshake nonce |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
📝 Walkthrough""" WalkthroughThis change introduces a new query parameter, Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found based on the objectives from the linked issues. Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/backend/src/constants.ts (1)
37-39: Query-param keyformatis extremely generic – consider a Clerk-specific prefixUsing the literal string
"format"forHandshakeFormatbreaks the naming pattern of the surrounding query parameters (all are Clerk-prefixed or otherwise very explicit) and could easily collide with existing parameters added by reverse proxies or middleware.
A more self-describing key such ashandshake_formator even re-using the earlier suggestionhandshake_modewould avoid ambiguity while remaining backward-compatible (the API can accept both).- HandshakeFormat: 'format', + HandshakeFormat: 'handshake_format',If you agree, remember to update
buildRedirectToHandshakeand the tests accordingly.
🧹 Nitpick comments (1)
.changeset/six-ears-wash.md (1)
1-6: Consider mentioning the actual query-param nameFor clarity to consumers reading the changelog, you might add something like “…by appending
format=nonceto the handshake URL” so they immediately know what to look for.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/six-ears-wash.md(1 hunks)packages/backend/src/constants.ts(1 hunks)packages/backend/src/tokens/__tests__/handshake.test.ts(2 hunks)packages/backend/src/tokens/handshake.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/backend/src/tokens/handshake.ts (1)
148-153: Unconditional inclusion is fine, but confirm API fallbackAppending the new parameter on every redirect is straightforward and keeps the client logic simple.
Please double-check that the backend will gracefully ignoreformat=noncewhen it decides to return a classic token‐based handshake so that older environments remain functional.packages/backend/src/tokens/__tests__/handshake.test.ts (2)
154-160: Good assertion for the new paramThe extra expectation ensures we never regress on adding the nonce signal. 👍
170-173: Covers the dev-browser branch as wellNicely catches the development-mode path; no issues spotted.
jacekradko
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/six-ears-wash.md (3)
5-5: Use a proper Markdown heading instead of bold textMarkdown-lint flags MD036 here. Replacing the bold line with a level-2 heading keeps the file consistent with other changesets and avoids lint noise.
-**Optimize handshake payload delivery with nonce-based fetching** +## Optimize handshake payload delivery with nonce-based fetching
23-26: Fix typo in example domain
ecxample.com➜example.com.-3. Handshake resolves → `307 ecxample.com` with `__clerk_handshake_nonce` cookie containing the nonce +3. Handshake resolves → `307 example.com` with `__clerk_handshake_nonce` cookie containing the nonce
30-31: Optional: add a clarifying commaMinor readability tweak; feel free to ignore if you prefer the current wording.
-Continues to work as before with direct payload delivery in cookies for optimal performance. +Continues to work as before, with direct payload delivery in cookies for optimal performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/six-ears-wash.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/six-ears-wash.md
[uncategorized] ~10-~10: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rs limit cookies to ~4KB, this severely restricted the practical size of session tokens, w...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~31-~31: Possible missing comma found.
Context: ... payloads ≤2KB):** Continues to work as before with direct payload delivery in cookies...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
.changeset/six-ears-wash.md
5-5: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/six-ears-wash.md (1)
10-11: Mixed tenses – change “restricted” → “restricts”Present-tense “limit” pairs naturally with present-tense “restricts”.
-… this severely restricted the practical size … +… this severely restricts the practical size …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/six-ears-wash.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/six-ears-wash.md
[uncategorized] ~10-~10: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rs limit cookies to ~4KB, this severely restricted the practical size of session tokens, w...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.changeset/six-ears-wash.md (1)
24-26: Step 3 wording is ambiguous
307 example.comdoesn’t tell the reader which endpoint the browser is redirected to. Spell out the full redirected URL or path (e.g./or the original page) so integrators know what to expect.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/six-ears-wash.md (1)
10-11: Minor grammar tweak for release-note polish“restricts” reads slightly better with the present-tense “Since …” lead-in.
- Since browsers limit cookies to ~4KB, this severely restricted the practical size of session tokens, + Since browsers limit cookies to ~4KB, this severely restricts the practical size of session tokens,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/six-ears-wash.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
.changeset/six-ears-wash.md (1)
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.511Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
🪛 LanguageTool
.changeset/six-ears-wash.md
[uncategorized] ~10-~10: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rs limit cookies to ~4KB, this severely restricted the practical size of session tokens, w...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.changeset/six-ears-wash.md (1)
1-3: Header syntax is project-compliant – no action neededSingle-quoted package names are approved by the repo’s Changesets setup (per prior discussion).
Looks good as-is.
Description
Send query string param to signal support for handshake nonce flow from current version of
@clerk/backendRelated: SDKI-979
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Tests